Files that changed from the base of the PR and between ec4986d and 8ffa694.

• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx (2 hunks)
• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/TargetReleaseTable.tsx (3 hunks)
• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/page.tsx (1 hunks)
• packages/api/src/router/environment.ts (5 hunks)
• packages/db/drizzle/0024_bouncy_bromley.sql (1 hunks)
• packages/db/drizzle/meta/_journal.json (1 hunks)
• packages/db/src/schema/environment.ts (2 hunks)

packages/db/drizzle/0024_bouncy_bromley.sql (1)

2-6: Review CASCADE DELETE behavior for audit compliance.

The current foreign key constraint will delete approval records when a user is deleted. This could be problematic for audit purposes where you need to maintain a history of who approved what, even if the user no longer exists.

Let's verify if this table is used for audit purposes:

Consider these alternatives:

1. Use SET NULL instead of CASCADE (requires removing NOT NULL constraint)
2. Use a soft delete for users to maintain referential integrity
3. Archive user data instead of deleting it

apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/page.tsx (2)

71-71: LGTM: Environment prop addition.

The addition of the environments prop to TargetReleaseTable aligns with the architectural changes, allowing the component to handle environment-specific data and approvals directly.

---

Line range hint 1-74: Verify the removal of approval handling logic.

The removal of policy approval handling from this component suggests an architectural shift where approval management has been moved to child components. This change could improve separation of concerns, but we should verify that the approval functionality is properly implemented in the new location.

Let's verify the approval handling implementation:

✅ Verification successful

Let me gather more information about the TargetReleaseTable component since the first command failed:

---

Approval handling has been correctly moved to child components

The removal of approval handling from the page component is a valid architectural improvement. The verification shows that:

• TargetReleaseTable fetches approvals via api.environment.policy.approval.byReleaseId and passes them to PolicyApprovalRow
• PolicyApprovalRow handles all approval-related actions (approve/reject) with proper mutations and UI feedback
• The addition of the environments prop to TargetReleaseTable is necessary as it's used to render environment-specific approvals

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that approval handling has been moved to TargetReleaseTable
# and ensure no approval-related functionality is lost

# Test 1: Check TargetReleaseTable for new approval handling
echo "Checking TargetReleaseTable implementation..."
rg -A 10 "PolicyApprovalRow|approval" --type typescript

# Test 2: Verify no orphaned approval-related code
echo "Checking for any orphaned approval-related code..."
ast-grep --pattern 'api.environment.policy.approval'

Length of output: 1616

---

Script:

#!/bin/bash
# Check the implementation of TargetReleaseTable and related components
echo "Checking TargetReleaseTable content..."
cat apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/TargetReleaseTable.tsx

echo "\nChecking PolicyApprovalRow content..."
cat apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx

Length of output: 10640

apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx (1)

17-18: Props refactoring improves type safety.

The props restructuring with optional user info and single environment improves type safety and better reflects the component's requirements.

packages/db/drizzle/meta/_journal.json (1)

172-178: LGTM! Migration journal entry is properly structured.

The new entry follows the established format and maintains sequential ordering.

Let's verify the corresponding migration file exists and contains the expected user tracking changes:

✅ Verification successful

Migration file verified and contains expected changes ✅

The migration file exists at the correct location and implements the expected schema changes:

• Adds user_id column (UUID) to environment_policy_approval table
• Sets up foreign key constraint referencing the user table with proper cascade behavior

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify migration file exists and contains user_id column addition

# Test 1: Check if migration file exists
fd "0024_bouncy_bromley.sql" --type f

# Test 2: Verify migration content includes user_id column addition
rg -A 5 "user_id" "packages/db/drizzle/0024_bouncy_bromley.sql"

Length of output: 503

packages/db/src/schema/environment.ts (1)

22-22: LGTM!

The import is correctly placed and follows the established import pattern.

packages/api/src/router/environment.ts (2)

34-34: LGTM: Import added for user schema

The addition of the user schema import aligns with the new user tracking functionality.

---

126-126: Verify impact of optional status on existing consumers

The changes to make status optional and include user information look good, but we should verify the impact on existing consumers of this API.

Also applies to: 137-137, 140-145, 152-152

✅ Verification successful

Making status optional is safe for existing consumers

Based on the codebase analysis:

1. The database schema maintains status as NOT NULL with a default value of 'pending'
2. Frontend components handle the status properly:
   • TargetReleaseTable.tsx safely filters and types the approvals data
   • FlowPolicyNode.tsx has proper null checks and status handling
3. The job dispatcher correctly checks status with environment_policy_approval?.status === "approved"

Making the status optional in the input schema while keeping it required in the database (with a default) is a safe change that won't impact existing consumers.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for direct usage of byReleaseId to verify consumers handle optional status
rg -A 5 "byReleaseId.*status.*" --type ts

Length of output: 43

---

Script:

#!/bin/bash
# Let's first find all references to byReleaseId function
rg -A 5 "byReleaseId" --type ts

# Also search for any router endpoints that might be consuming this
rg -A 5 "router\.(get|post|put|delete).*release.*approval" --type ts

# Let's also check the schema usage
ast-grep --pattern 'z.enum(["pending", "approved", "rejected"])'

Length of output: 7179

---

Script:

#!/bin/bash
# Let's check how the approval data is being used in the frontend components
rg -B 5 -A 10 "environmentApprovals" --type tsx

# Also check for any direct status handling
rg -B 2 -A 5 "approval.*status" --type tsx

Length of output: 148

---

Script:

#!/bin/bash
# Let's check how the approval data is being used in the frontend components, without file type restriction
rg -B 5 -A 10 "environmentApprovals"

# Also check for any direct status handling
rg -B 2 -A 5 "approval.*status"

# Let's also check the component that uses the byReleaseId query
rg -B 10 -A 10 "policy\.approval\.byReleaseId\.useQuery"

Length of output: 45790

apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/TargetReleaseTable.tsx (5)

72-97: Great implementation of collapsible environment rows

The use of useState for controlling the expansion of environment rows and the clean rendering logic enhance the user experience by making the interface more interactive and intuitive.

---

100-191: Conditional rendering is handled correctly

The conditional rendering of job details based on the isExpanded state is implemented well. The mapping over jobs to display each job's information ensures that users can access detailed data when needed.

🧰 Tools

🪛 Biome

[error] 101-190: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

---

89-96: Proper handling of policy approvals

Displaying policy approvals using the PolicyApprovalRow component with appropriate props ensures that approval statuses are accurately represented for each environment.

---

148-168: Robust handling of external links

The code safely renders external links with the correct target and rel attributes, which is important for security and preventing potential vulnerabilities like reverse tabnabbing.

---

Line range hint 196-221: Efficient rendering of TargetReleaseTable

Iterating over environments to render CollapsibleTableRow components makes the TargetReleaseTable dynamic and scalable for multiple environments.

🧰 Tools

🪛 Biome

[error] 101-190: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

Files that changed from the base of the PR and between 8ffa694 and 132540a.

• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/FlowPolicyNode.tsx (3 hunks)
• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx (3 hunks)
• packages/db/drizzle/meta/_journal.json (1 hunks)

• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx
• packages/db/drizzle/meta/_journal.json

Files that changed from the base of the PR and between 132540a and c409dc8.

• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx (3 hunks)
• apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/TargetReleaseTable.tsx (3 hunks)
• packages/api/src/router/environment.ts (5 hunks)

apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/PolicyApprovalRow.tsx (5)

6-6: LGTM: Type changes improve type safety.

The updated types properly reflect the nullable user information and single environment requirement.

Also applies to: 16-17

---

32-34: LGTM: Clean variable declarations.

The destructuring and data fetching logic is well-organized.

---

42-42: LGTM: Improved user feedback messages.

The toast messages now properly reference the environment name and provide clear feedback.

Also applies to: 53-53

---

58-69: LGTM: Concise action handlers.

The mutation handlers are well-structured and maintain consistent behavior.

---

72-97: 🛠️ Refactor suggestion

Consider extracting status-specific components.

I agree with the previous review comment about moving these to their own components. This would improve maintainability and reusability.

Consider this structure:

const PendingActions: React.FC<{ onApprove: () => void; onReject: () => void }> = ...
const ApprovalStatus: React.FC<{ status: string; userName?: string }> = ...

// In main component
return (
  <div className="flex items-center gap-2 rounded-md text-sm">
    {status === "pending" ? (
      <PendingActions onApprove={handleApprove} onReject={handleReject} />
    ) : (
      <ApprovalStatus status={status} userName={approval.user?.name} />
    )}
  </div>
);

apps/webservice/src/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/TargetReleaseTable.tsx (3)

Line range hint 3-34: LGTM! Well-structured type definitions and imports.

The type definitions are clear and properly typed, with good organization of imports.

---

192-216: LGTM! Clean implementation of the table wrapper.

The component is well-structured and properly handles the mapping of environments to CollapsibleTableRow components.

---

97-187: ⚠️ Potential issue

Remove unnecessary Fragment wrapper.

The Fragment wrapper around the conditional rendering is redundant since it contains only one child.

Replace the Fragment with the direct conditional rendering:

-      {isExpanded && (
-        <>
+      {isExpanded && jobs?.map((job, idx) => {
         // ... existing code
-        </>
-      )}
+      })}

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome

[error] 97-186: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

packages/api/src/router/environment.ts (3)

34-34: LGTM: User schema import added correctly

The import is properly placed and will support the new user tracking functionality.

---

Line range hint 126-152: LGTM: Enhanced query flexibility and user tracking

The changes improve the query flexibility by making the status filter optional and properly integrate user tracking through a left join. The nullable user relationship aligns with the PR objective of tracking approvers/rejectors.

---

225-235: LGTM: Consistent user tracking in rejection process

The changes maintain consistency with the approval process by implementing the same user tracking pattern while preserving the existing transaction and job cancellation logic.